Skip to content

Conversation

@mwitcpalek
Copy link
Contributor

No description provided.

Copy link
Contributor Author

@mwitcpalek mwitcpalek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review comments

return talonFXConfiguration;
}

// These are all wrong right now because we don't have any actual info
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move these above the talonFXconfig getter for consistency across constants files

// public static Angle Level1 = ;

// Disables the TalonFX by setting it's voltage to zero. Not very shocking.
public static TalonFXConfiguration disableTalon() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we needed to disable the fwd limit switch as part of disabling output if it wasn't in a safe region
This could just return a VoltageConfig instead of a full talon config - if you don't change you should get your talonFX config from the one above and just modify the voltage config, not start from a blank one


public class BiscuitConstants {

public static TalonFXConfiguration talonConfiguration() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a duplicate of line 53? If so get rid of this

}

public void setPosition(Angle position) {
setPoint = position.plus(BiscuitConstants.kZero);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that we have an encoder 1:1 we can actually write to the encoder in the talonFX so we don't actually have to save off zeroed position and use that as an offset

public void updateInputs(BiscuitIOInputs inputs) {
BaseStatusSignal.refreshAll(velocity, position, fwdLimitSwitch);
inputs.velocity = velocity.getValue();
inputs.position = position.getValue().minus(BiscuitConstants.kZero);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be able to convert this to just getValue()

public void zero() {
didZero = false;
if (fwdLimitSwitchOpen == true) {
Angle pos = position.getValue();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can actually write to the encoder position now so need to add subtraction logic based on zero constant and current pos from a remote encoder

Right now you have no way to get that remote encoder (we need new SW api) so just use get position for now


@Override
public void periodic() {
io.updateInputs(inputs);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to add processInputs()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add logging of the current setpoint to the periodic - see example subsystem for a reference
Put them under "Biscuit/"

return Set.of(new Measure("Is Biscuit Finished", () -> isFinished() ? 1.0 : 0.0));
}

public void zero() {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May want to use your "didZero" in the i/o layer for passing info up to robotState

@mwitcpalek mwitcpalek merged commit e121375 into main Jan 21, 2025
1 check passed
@mwitcpalek mwitcpalek deleted the biscuit branch January 30, 2025 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants